- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 311
 
Change default file format to 1.8 #5949
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| 
           There were no CMake changes so need to see the failing instance.  | 
    
| 
           One of the first workflows failing had two test failures:  | 
    
| 
           Current CI CMake 4 arm64 Workflows / MacOS Clang-Release-- has these tests failing: The following tests FAILED:  | 
    
| 
           Searching the logs for the failing tests shows this for Fparams java test:  | 
    
          
 
 This was the failure that I thought was cmake related - those two test files were missing. It's possible that my working directory was messed up. I deleted it and tried again and saw the same two tests fail, but it's possible I didn't notice they failed for a different reason, as Jordan saw the same two tests fail with an expected output mismatch. I fixed the mismatch so they should work now. Do they still fail?  | 
    
          
 Yes. You can easily duplicate it using my GH Action script:  | 
    
          
 You may need to use   | 
    
| 
           These failures appear to be preexisting bugs that were exposed by the change, and not simply tests expecting the oldest file format. I'm trying to track them down but it might take some time.  | 
    
| 
           The 2 failures with multi were due to issues with the test code. The family failure was due to weirdness in how the family driver handled the user block. My position is the family driver shouldn't handle it at all, but I'm willing to hear other opinions. Either way the way it handled it before was wrong.  | 
    
        
          
                release_docs/CHANGELOG.md
              
                Outdated
          
        
      | 
               | 
          ||
| ### Fixed problems with family driver and user block | ||
| 
               | 
          ||
| When using a user block with the family driver, the driver would inappropriately subtract the user block size for each member file when calculating member eoas. This could cause a failure when an addresse overflowed the calculated eoa. The driver would also add the user block size when returning the eof. Modified the family driver to not consider the user block, as it is handled by the H5FD layer. The user block now spans the first x bytes of the family array, for example a 4 KiB user block with 3 KiB member size will take up the entire first member and the first 1 KiB of the second. This may cause compatibility issues with preexisiting family files with user blocks, though the way it worked before was inconsistent if it worked at all. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| When using a user block with the family driver, the driver would inappropriately subtract the user block size for each member file when calculating member eoas. This could cause a failure when an addresse overflowed the calculated eoa. The driver would also add the user block size when returning the eof. Modified the family driver to not consider the user block, as it is handled by the H5FD layer. The user block now spans the first x bytes of the family array, for example a 4 KiB user block with 3 KiB member size will take up the entire first member and the first 1 KiB of the second. This may cause compatibility issues with preexisiting family files with user blocks, though the way it worked before was inconsistent if it worked at all. | |
| When using a user block with the family driver, the driver would inappropriately subtract the user block size for each member file when calculating member eoas. This could cause a failure when an address overflowed the calculated eoa. The driver would also add the user block size when returning the eof. Modified the family driver to not consider the user block, as it is handled by the H5FD layer. The user block now spans the first x bytes of the family array, for example a 4 KiB user block with 3 KiB member size will take up the entire first member and the first 1 KiB of the second. This may cause compatibility issues with preexisting family files with user blocks, though the way it worked before was inconsistent if it worked at all. | 
        
          
                release_docs/CHANGELOG.md
              
                Outdated
          
        
      | 
               | 
          ||
| ### Fixed problems with family driver and user block | ||
| 
               | 
          ||
| When using a user block with the family driver, the driver would inappropriately subtract the user block size for each member file when calculating member eoas. This could cause a failure when an addresse overflowed the calculated eoa. The driver would also add the user block size when returning the eof. Modified the family driver to not consider the user block, as it is handled by the H5FD layer. The user block now spans the first x bytes of the family array, for example a 4 KiB user block with 3 KiB member size will take up the entire first member and the first 1 KiB of the second. This may cause compatibility issues with preexisiting family files with user blocks, though the way it worked before was inconsistent if it worked at all. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| When using a user block with the family driver, the driver would inappropriately subtract the user block size for each member file when calculating member eoas. This could cause a failure when an addresse overflowed the calculated eoa. The driver would also add the user block size when returning the eof. Modified the family driver to not consider the user block, as it is handled by the H5FD layer. The user block now spans the first x bytes of the family array, for example a 4 KiB user block with 3 KiB member size will take up the entire first member and the first 1 KiB of the second. This may cause compatibility issues with preexisiting family files with user blocks, though the way it worked before was inconsistent if it worked at all. | |
| When using a user block with the family driver, the driver would inappropriately subtract the user block size for each member file when calculating member eoas. This could cause a failure when an address overflowed the calculated eoa. The driver would also add the user block size when returning the eof. Modified the family driver to not consider the user block, as it is handled by the H5FD layer. The user block now spans the first x bytes of the family array, for example a 4 KiB user block with 3 KiB member size will take up the entire first member and the first 1 KiB of the second. This may cause compatibility issues with preexisting family files with user blocks, though the way it worked before was inconsistent if it worked at all. | 
| try { | ||
| // Create file FILE1 | ||
| file1 = new H5File(FILE1, H5F_ACC_EXCL); | ||
| file1 = new H5File(FILE1, H5F_ACC_TRUNC); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the change in file access mode here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test is creating a new file. H5F_ACC_EXCL causes it to fail if the file already exists, which is what was happening. It was failing silently before, but with the 1.8 format it causes issues due to a fapl mismatch - see #5954
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Fine either way, though previously the issues with this test involved the fact that it left a file open unintentionally and also forgot to delete it, so I wonder if that isn't the case here as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is at least partly due to it simply using HDremove() to remove the test files, which obviously doesn't work with the family file driver. Something that should be fixed for sure, but not within the scope of this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address typos
| 
           We talked with Quincey about the family driver changes and he agreed with this approach.  | 
    
| 
           Matt's review was dismissed by the commit of a typo fix in CHANGELOG.md  | 
    
2 remaining test failures seemingly related to cmake. Adding this PR in the hopes that someone better at cmake than me can help.
Closes #5315
Important
Change default file format to 1.8 across various tests and examples, updating file creation and access logic accordingly.
H5Pfapl.c.h5ex_g_compact.candtest_file_image.c.cache_tagging.c,dtypes.c, andlinks.c.tools/test/misc/expected/*.lsfiles to reflect new file format locations.test_file_image.candcache_tagging.cto accommodate format changes.test_file_image.c.test_file_image.c.This description was created by
 for 85332ed. You can customize this summary. It will automatically update as commits are pushed.